Skip to content

Refactor main loop in raft node of five node example to improve readability#549

Open
hhwyt wants to merge 2 commits into
tikv:masterfrom
hhwyt:master
Open

Refactor main loop in raft node of five node example to improve readability#549
hhwyt wants to merge 2 commits into
tikv:masterfrom
hhwyt:master

Conversation

@hhwyt

@hhwyt hhwyt commented Sep 4, 2024

Copy link
Copy Markdown

What problem does this PR solve?

The examples/five_mem_node main loop was hard to read and maintain because control flow and polling logic were densely nested. In addition, CI surfaced minor compilation/style issues in other files.

What is changed?

  • Refactored the node thread main loop in examples/five_mem_node/main.rs for clearer control flow and readability.
  • Applied follow-up compile/style fixes in several files (src/lib.rs, src/quorum/majority.rs, src/raw_node.rs, src/storage.rs, src/util.rs).

Impact

  • No behavior changes intended for raft algorithm logic.
  • Primary goal is readability/maintainability of example code plus CI cleanliness.

Test plan

  • Local/CI validation should include:
    • cargo test
    • cargo test --examples

@hhwyt hhwyt force-pushed the master branch 2 times, most recently from cd322d7 to d133b6d Compare September 5, 2024 03:02
- Adjusted the indentation in documentation comments to ensure proper formatting.
- Replaced legacy numeric constants with associated constants.
- Removed unnecessary import of legacy numeric constants.

Signed-off-by: hhwyt <hhwyt1@gmail.com>
@hhwyt hhwyt changed the title Refactor main loop in raft node thread for better readability Refactor main loop code in raft node thread for better readability Sep 5, 2024
@hhwyt

hhwyt commented Sep 5, 2024

Copy link
Copy Markdown
Author

/cc @glorv @overvenus PTAL.

Comment thread src/lib.rs

1. Check whether `messages` is empty or not. If not, it means that the node will send messages to
other nodes:
other nodes:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary space?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary space?

This is a compiler error from CI, so I made this change.

CI output:

error: doc list item missing indentation
   --> src/lib.rs:204:1
    |
204 | other nodes:
    | ^
    |
    = help: if this is supposed to be its own paragraph, add a blank line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
note: the lint level is defined here
   --> src/lib.rs:478:9
    |
478 | #![deny(clippy::all)]
    |         ^^^^^^^^^^^
    = note: `#[deny(clippy::doc_lazy_continuation)]` implied by `#[deny(clippy::all)]`
help: indent this line
    |
204 |    other nodes:
    | +++

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hhwyt hhwyt changed the title Refactor main loop code in raft node thread for better readability Refactor main loop in raft node of five node example to improve readability Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants